Skip to content

Conversation

@Pat-pGuo
Copy link

@Pat-pGuo Pat-pGuo commented Nov 29, 2024

For review:

  • implement leaf datatypes
  • implement structural datatypes
  • make dictionary.Attribute.encode() and decode() datatype agnostic
  • make packet.DecodePacket() datatype agnostic
  • existing unittests fixes

Work in progress:

@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch from 9eafd73 to 8a8a4d9 Compare December 2, 2024 21:11
':'.join(map('{0:02x}'.format, struct.unpack('!6B', raw)))

def print(self, attribute, decoded):
return decoded

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the "print" functions are supposed to do here.

The requirement for print is "print this value to an ASCII string". The class also needs a related parse routine. which takes an ASCII string, and returns a python structure / dict containing the attribute

This functionality lets us do the following tests:

decode HEX -> python stuff
print python stuff - ASCII

Human reads the ASCII and manually checks it agains the HEX to be sure it's OK.

We can then do

parse ASCII -> python stuff
encode python stuff -> HEX

which should then result in the same HEX output as above.

Once the unit tests have been verified manually (or copied from FreeRADIUS), the tests can be run in CI. The tests then ensure that:

a) the decoder can handle the packets correctly
b) the ASCII output doesn't change
c) the parser can read ASCII output
d) the encoder can encode things correctly

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes, the print function is returning an ASCII string. It just so happens that Pyrad has been storing these datatypes as strings, thus I simply return the value (which is an ASCII string).

cls.integer64 = Integer64()
cls.ipaddr = Ipaddr()
cls.ipv6addr = Ipv6addr()
cls.ipv6prefix = Ipv6prefix()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an IPv4 prefix type?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per discussions with @arr2036, that's planned for the next PR (which will contain all of the datatypes not currently in Pyrad). This PR is for restructuring existing datatypes.

@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch 2 times, most recently from 03e85ad to 9a1c6b5 Compare December 6, 2024 18:10
@Pat-pGuo Pat-pGuo force-pushed the feature/dynamic-datatype-loader branch from 9a1c6b5 to 60349c0 Compare December 6, 2024 18:35
@Pat-pGuo Pat-pGuo marked this pull request as ready for review December 6, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants